Conversation
- Add `storage_name` column to `feed_audit` and `feed_audit_detail` - `main_repo_audit.pl` now requires a `--storage_name` parameter - Expected values `s3-truenas-ictc` and `s3-truenas-macc` - `HTFeed/Storage/LocalPairtree.pm` and `HTFeed/StorageAudit.pm` also write `feed_audit.storage_namd` and `feed_audit_detail.storage_name`, respectivelt, but I'm not sure what values that can or should provide - Add a couple of happy path tests for the script as a whole.
- New code is truenas_audit.pl - TODO: several FIXMEs in the code, possible leftovers from main_repo_audit.pl inside truenas_audit.pl
aelkiss
left a comment
There was a problem hiding this comment.
I've looked at this some; I want to look at the tests more, not sure I'll get to that today though.
aelkiss
left a comment
There was a problem hiding this comment.
Tests look good; see comments about the backticks. Not critical, but maybe worth considering.
- Refactor some of the path logic in `truenas_audit.t`
|
@aelkiss Since the scope of this has changed to focus on |
aelkiss
left a comment
There was a problem hiding this comment.
There are definitely some things here to think about in the future:
- how we use the checksums saved in
feed_storage - questions about
feed_auditgoing forward - in the future, merging this with
StorageAuditand making it more testable
I think this will get us what we need for now, and we can make some follow-up issues for the remaining issues.
bin/audit/truenas_audit.pl
Outdated
|
|
||
|
|
||
| #insert | ||
| execute_stmt( |
There was a problem hiding this comment.
Maybe wait to do this until after the file checks? Maybe worth comparing to the backup audits to see behavior -- that is, do we update lastchecked when we see it or after we verify some part of it?
There was a problem hiding this comment.
I moved this a little closer to the zipcheck call. At that point we have at least checked the file count and looked for BAD_FILE, BARCODE_MISMATCH, and symlink issues.
t/truenas_audit.t
Outdated
| is($db_data->[0]->{storage_name}, $storage_name, 'correct storage_name'); | ||
| ok($db_data->[0]->{zip_size} > 0, 'nonzero zip_size'); | ||
| ok($db_data->[0]->{mets_size} > 0, 'nonzero mets_size'); | ||
| ok(!defined $db_data->[0]->{saved_md5sum}, 'not defined saved_md5sum'); |
There was a problem hiding this comment.
Do we want it to save this? Is it worth using if it's there? Probably fine for now to leave as-is, but maybe worth thinking about in the future or otherwise making it more consistent with what we do for the backup auditing.
There was a problem hiding this comment.
After looking at some of the other code I changed the behavior to record the actual zip md5, so now that particular test requires the value to be defined.
…heck() - Return the actual md5sum by reference from zipcheck() for insertion into feed_storage.saved_md5sum
storage_namecolumn tofeed_auditandfeed_audit_detailmain_repo_audit.plnow requires a--storage_nameparameters3-truenas-ictcands3-truenas-maccHTFeed/Storage/LocalPairtree.pmandHTFeed/StorageAudit.pmalso writefeed_audit.storage_namdandfeed_audit_detail.storage_name, respectivelt, but I'm not sure what values that can or should provide